Skip to content

Conversation

@ElodinLaarz
Copy link

In the specification, deletes are listed as optional for SAMPLE subscriptions, but this is not the case when the suppress_redundant flag is present.

Normally, if a client does not get a notification for a leaf over a SAMPLE subscription, they can use some form of cache invalidation to get rid of stale values. Since suppress_redundant could cause some notifications to be withheld for arbitrary lengths of time, the client cannot decide when the data should be invalidated; so, the server must send deletes.

@dplore dplore moved this to Ready to discuss in OC Operator Review Apr 29, 2025
@dplore
Copy link
Member

dplore commented Apr 29, 2025

@ElodinLaarz thanks for this pull request. Can you quote any operational use cases where this is a problem today? This does look like a real gap in the logic, but I would like to hear of some real operational use cases where this is a problem before changing the specification.

@dplore
Copy link
Member

dplore commented May 1, 2025

In the OC Community meeting, this older thread was brought up for historical context around suppress_redundant deprecation: openconfig/gnmi#114

@ElodinLaarz
Copy link
Author

I am not opposed to removing suppress_redundant. (I am also not sure if anyone has properly implemented it. :))

The additions here are merely to highlight that if suppress redundant is employed and deletes are not included, then a client can be left with indefinitely stale data.

If, for example, you collect interface data with a SAMPLE subscription with suppress_redundant enabled, then if the interface is subsequently deleted, then you have no way of distinguishing the lack of updates for the data with data that has remained constant for an indefinite period of time (even if the heartbeat interval is enabled, you don't know about particular, keyed interfaces).

@jsterne
Copy link

jsterne commented May 1, 2025

In the community meeting several folks expressed that doing anything "stateful" for SAMPLE subscriptions is expensive on a network device and more in the spirit of being attached to ON_CHANGE subscriptions. Both suppress_redundant and delete notifications in a SAMPLE subscriptions have significant cost and impact to implementations - it would be good to avoid those for SAMPLE, and focus on ON_CHANGE for use cases that require async notification and "stateful" type notification.

@ElodinLaarz
Copy link
Author

A possible reason for wanting suppress_redundant to exist could be to get ON_CHANGE behavior with a minimum update interval.

Importantly, if you subscribe to a path (e.g. Counters) ON_CHANGE, then of course, you do not get the counters whenever they actually increment (this could be billions of times per second!). Instead, the device has a 'minimal' cadence in which the OS is updated from the source (every 1 second, 10 seconds, etc.).

But this expected cadence is opaque to the subscriber.

I spoke with Darren offline, and it is possible that /interfaces/interface/rates/config/load-interval would allow this interval to be configured (and subsequently read by the client), but this does not change the fact that the gNMI Subscribe Client has no control over this interval. It requires a configuration operation on the device.

So, there is a use-case for suppress_redundant existing, but if we have no knowledge of it being implemented, and it seems infeasible to do it, it is not particularly important for this gap in logic (which is still true!) to be updated in the spec.

@gwizdms
Copy link
Contributor

gwizdms commented May 5, 2025

Another case in support of the deprecation of suppress redundancy then delete is no longer an issue. openconfig/gnmi#114.

@dplore dplore moved this from Ready to discuss to Waiting for author in OC Operator Review May 6, 2025
@dplore
Copy link
Member

dplore commented May 6, 2025

Reviewed in May 6, 2025 OC operators meeting. It's suggested to instead proposal a deprecation of this field from the gnmi specification as it appears infeasible to implement at scale and to avoid this ambiguity it introduces. @ElodinLaarz we would appreciate it if you could make that proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Waiting for author

Development

Successfully merging this pull request may close these issues.

4 participants